Add axes and get_axis_dim to BaseGrid and other cleanup - #2080
Merged
Conversation
Wasn't used anymore in the codebase
Overloaded behaviour that wasn't justified/used in the rest of the codebase.
Was in environment.yaml file - wasn't in the pyproject.toml file though
These variables mainly apply to structured grids. With the new API, they aren't needed in the codebase (we can add this on the repective grid classes if needed).
Also added some simple tests in test_uxgrid.py
VeckoTheGecko
commented
Jul 10, 2025
| @abstractmethod | ||
| def get_axis_dim(self, axis: str) -> int: | ||
| """ | ||
| Return the dimensionality (number of cells/edges) along a specific axis. |
Contributor
Author
There was a problem hiding this comment.
Suggested change
| Return the dimensionality (number of cells/edges) along a specific axis. | |
| Return the dimensionality (number of cells/faces) along a specific axis. |
I think that this should be cells/faces and not 'edges'. This is what we ravel against - so edges don't really apply (since particles can't occupy that)
Contributor
We can remove the helper function now that we have the understanding in UxGrid that the vertical position is explicitly relative to the vertical layers. |
erikvansebille
approved these changes
Jul 11, 2025
erikvansebille
left a comment
erikvansebille
left a comment
Member
There was a problem hiding this comment.
Looks good; one question below
VeckoTheGecko
enabled auto-merge (rebase)
July 11, 2025 10:55
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes:
axesandget_axis_dim()to the BaseGrid class with corresponding implementations in the UxGrid and XGrid classes. Also moved ravel/unravel logic to the BaseGrid class (fixes Addingaxesandget_axis_dimto BaseGrid #2056)lon, lat, depthfrom the Field classOpen questions:
@fluidnumerics-joe, the implementation of
Field.depthdiffered fromUxGrid.depth- with the former using a helper functionget_vertical_location_from_dims. Is this function still needed, or can we remove it?Chose the correct base branch (
mainfor v3 changes,v4-devfor v4 changes)Fixes Adding
axesandget_axis_dimto BaseGrid #2056Added tests
Added documentation